Review: template-simple-modifiers

Compared against ember-template-lint simple-modifiers.js.

General Correctness

1. Faithful port: The logic correctly mirrors the original ETL rule. The original checks SubExpression nodes where node.path.original === 'modifier', validates the first param is a StringLiteral or PathExpression, and reports when missing or invalid. The ESLint port does the same with the appropriate Glimmer AST node types. Looks good.

2. Error message: Matches the original exactly. Good.

3. meta.messages is empty but inline messages are used: The messages: {} object is empty, yet context.report() uses inline message strings. This is inconsistent with other rules in the plugin. Consider defining a messageId in messages and using messageId in context.report() calls instead of inline message. This would also make tests more robust (testing messageId rather than matching full strings).

4. Tests: Good coverage for both gjs and hbs modes. The test cases match the original ETL test scenarios well.

5. Missing recommended: false: The docs metadata has recommended: false but this is fine -- just noting consistency.

Scope Analysis (gjs/gts)

This rule checks node.path.original === 'modifier' to match the built-in modifier helper. In gjs/gts files, a user could shadow modifier with a local import:

import { modifier } from 'some-custom-lib';
// This 'modifier' is not Ember's built-in modifier helper

The rule should ideally use context.sourceCode.getScope(node) to verify that modifier is not shadowed by a local JS binding before reporting. Without this check, the rule would incorrectly flag custom functions named modifier that happen to be used in sub-expressions. That said, this is an edge case since the modifier keyword is quite specific to Ember's API, so this is a low-severity concern.

🤖 Automated review comparing with ember-template-lint source